Repository navigation
feat(cloudfetch): apply download settings to kernel sessions - #616
aakash-saravanan-db wants to merge 4 commits into
Conversation
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
Signed-off-by: Aakash Saravanan <aakash.saravanan@databricks.com>
![peco-review-bot[bot]](https://avatars.githubusercontent.com/in/3801177?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: 1 Low
The change looks correct, with one low finding: the session-open validation fires even when a raw chunk override makes the client setting irrelevant. The mapping, case-insensitive override detection, per-session snapshot and caller-config immutability are tested well. I couldn't check whether the native kernel actually reads cloudfetch_max_chunks_in_memory from sessionConf, because the kernel source isn't in this checkout.
There was a problem hiding this comment.
Verdict: 1 Low
Looks good, with one low concern. The kernel now rejects an invalid cloudFetchConcurrentDownloads even when an explicit raw override or disabled CloudFetch means the value is never used. I couldn't check the claims about native-kernel behaviour (clamping above 256, invalid raw overrides warning and falling back to 16) because the kernel source isn't in this checkout.
Feature or gap
Node.js applications could set
cloudFetchConcurrentDownloadson Thrift connections, but kernel sessions ignored it.What this PR addresses
Snapshot the existing setting at session open, default 10 (previously 16), and preserve explicit raw chunk overrides. Values other than positive safe integers raise
HiveDriverErrorat session open. Document when mutations take effect and what the chunk window bounds.Dependencies
Existing native kernel APIs; no API change. Thrift parity with
runAsync: truerequires the kernel CloudFetch link-prefetch fix (#466), so a Node.js release must use a native build that includes it. Without that fix, kernel sessions usingrunAsync: trueare slower than Thrift on large multi-page CloudFetch results.Tests and validation
Build, type-check, repository lint and 440 kernel tests pass. Packaged-driver checks verify controls, errors, precedence, complete results and constrained consumption. With a native built from kernel
eabe1a54, five alternating measurements per side after equal warmups give DBSQL Preview submission-through-drain medians for 3M ordered rows: default path Thrift 6.030 s → kernel 5.363 s (-11.1%);runAsync: trueThrift 6.016 s → kernel 5.323 s (-11.5%).